Skip to content

add umfPoolTrimMemory #1318

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Aug 5, 2025
Merged

add umfPoolTrimMemory #1318

merged 1 commit into from
Aug 5, 2025

Conversation

bratpiorka
Copy link
Contributor

@bratpiorka bratpiorka commented May 15, 2025 •
edited
Loading

implement umfPoolTrimMemory

lplewa
lplewa reviewed May 15, 2025
View reviewed changes
vinser52
vinser52 reviewed May 15, 2025
View reviewed changes
ldorau
ldorau requested changes May 19, 2025
View reviewed changes
@bratpiorka bratpiorka changed the title [POC] add umfDisjointPoolTrimMemory [POC] trim Jun 16, 2025
@bratpiorka bratpiorka force-pushed the rrudnick_dp_trim branch 3 times, most recently from 4a3b48f to 4dee8a2 Compare June 18, 2025 07:25
@bratpiorka bratpiorka force-pushed the rrudnick_dp_trim branch 2 times, most recently from a699662 to c151a5c Compare July 21, 2025 13:26
@bratpiorka bratpiorka changed the title [POC] trim add umfDisjointPoolTrimMemory Jul 21, 2025
@bratpiorka bratpiorka marked this pull request as ready for review July 21, 2025 13:28
@bratpiorka bratpiorka requested a review from a team as a code owner July 21, 2025 13:28
KFilipek
KFilipek requested changes Jul 21, 2025
View reviewed changes
src/pool/pool_disjoint.c Outdated
Comment on lines 1138 to 1140
if (hPool == NULL) {
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you introduce UMF_CHECK above, please apply it here as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed to assert as this is checked in the upper layer

test/memoryPoolAPI.cpp Outdated
Comment on lines 330 to 332
// TODO enable jemalloc pool tests
//poolCreateExtParams{umfJemallocPoolOps(), nullptr, nullptr,
// &BA_GLOBAL_PROVIDER_OPS, nullptr, nullptr}));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct indentation and add information when it should be enabled

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed + created an issue #1455

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that #1455 is fixed

lukaszstolarczuk
lukaszstolarczuk reviewed Jul 22, 2025
View reviewed changes
@lukaszstolarczuk lukaszstolarczuk changed the title add umfDisjointPoolTrimMemory add umfPoolTrimMemory Jul 22, 2025
@bratpiorka bratpiorka force-pushed the rrudnick_dp_trim branch 2 times, most recently from 6d877f9 to c523215 Compare July 22, 2025 10:56
@bratpiorka bratpiorka force-pushed the rrudnick_dp_trim branch 7 times, most recently from 02c6e96 to c2a5994 Compare July 23, 2025 08:48
lplewa
lplewa requested changes Jul 23, 2025
View reviewed changes
@bratpiorka bratpiorka requested a review from lplewa August 5, 2025 07:53
@bratpiorka
Copy link
Contributor Author

@KFilipek @lukaszstolarczuk @lplewa please re-review

lplewa
lplewa reviewed Aug 5, 2025
View reviewed changes
lplewa
lplewa reviewed Aug 5, 2025
View reviewed changes
lplewa
lplewa reviewed Aug 5, 2025
View reviewed changes
@bratpiorka bratpiorka force-pushed the rrudnick_dp_trim branch 2 times, most recently from 9b7d33e to 33878b6 Compare August 5, 2025 12:59
lukaszstolarczuk
lukaszstolarczuk reviewed Aug 5, 2025
View reviewed changes
@KFilipek KFilipek merged commit 047432a into oneapi-src:main Aug 5, 2025
75 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vinser52 vinser52 vinser52 left review comments

@KFilipek KFilipek KFilipek approved these changes

@ldorau ldorau ldorau approved these changes

@lplewa lplewa lplewa approved these changes

@lukaszstolarczuk lukaszstolarczuk lukaszstolarczuk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@bratpiorka @KFilipek @vinser52 @ldorau @lplewa @lukaszstolarczuk